Enforce RFC 5746 renegotiation_info in TLS 1.2 client by default#10984
Open
embhorn wants to merge 4 commits into
Open
Enforce RFC 5746 renegotiation_info in TLS 1.2 client by default#10984embhorn wants to merge 4 commits into
embhorn wants to merge 4 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR makes TLS 1.2 client builds enforce RFC 5746 / RFC 9325 secure renegotiation signaling by default: the client advertises an empty renegotiation_info extension and aborts the initial handshake with a fatal handshake_failure if the server omits the extension, while providing compile-time and runtime opt-outs.
Changes:
- Enable the RFC 5746 renegotiation_info initial-handshake check by default (when
HAVE_SERVER_RENEGOTIATION_INFOis available), and send a fatalhandshake_failurewhen the ServerHello omits the extension. - Add context-level SCR check controls (
wolfSSL_CTX_get/set_scr_check_enabled) and wire context → object inheritance for the setting. - Add/adjust tests around default behavior, opt-outs, and “advertise-only” refusal of server-initiated renegotiation; add a
--enable-legacy-server-connectconfigure option.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
wolfssl/ssl.h |
Exposes new context-level SCR check enable/disable APIs and updates build guards for SCR check APIs. |
wolfssl/internal.h |
Adds advertiseOnly state to secure renegotiation tracking and adds ctx/object SCR enforcement bitfields. |
src/internal.c |
Implements advertise-only behavior, defaults/inheritance for SCR enforcement, and sends fatal alert on missing ServerHello extension. |
src/ssl_api_ext.c |
Enforces advertise-only refusal in _Rehandshake() and adds ctx-level SCR check accessors. |
src/ssl.c |
Ensures secure renegotiation advertising state is rebuilt appropriately when reusing an object after wolfSSL_clear(). |
src/tls13.c |
Re-establishes renegotiation_info advertising on TLS 1.3 ClientHello send when needed (for downgrade paths / reuse). |
tests/api.c |
Expands SCR enforcement tests (default-on, strip extension failure, ctx/object toggles) and adds alert-history assertions. |
tests/api/test_tls_ext.h |
Adds new test prototype for advertise-only HelloRequest refusal. |
tests/api/test_tls_ext.c |
Adds new test ensuring advertise-only clients refuse server HelloRequest renegotiation and continue application data. |
configure.ac |
Adds --enable-legacy-server-connect and rejects it when combined with --enable-harden-tls. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #10984
Scan targets checked: wolfcrypt-rs-bugs, wolfssl-bugs, wolfssl-src
Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Findings are non-blocking.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
A default-build TLS 1.2 client previously neither advertised the renegotiation_info extension nor verified that the server echoed it. The RFC 5746 4.1 / RFC 9325 check already existed but was compiled only under WOLFSSL_HARDEN_TLS.
In this PR, we make the client advertise the empty renegotiation_info extension and abort with a fatal handshake_failure alert when the initial ServerHello omits it, in any build with HAVE_SERVER_RENEGOTIATION_INFO (on by default). Provide opt-outs for legacy peers: the compile-time WOLFSSL_ALLOW_LEGACY_SERVER_CONNECT (--enable-legacy-server-connect), the
existing runtime wolfSSL_set_scr_check_enabled(), and a new wolfSSL_CTX_set_scr_check_enabled(). Reject --enable-legacy-server-connect together with --enable-harden-tls, since that combination violates RFC 9325 3.5.
Fixes zd22162
This is a draft message for the changelog:
Testing
Add unit tests for the default advertise and enforcement path, the runtime and context opt-outs, and the advertise-only HelloRequest refusal.
Checklist